Dpx 693 pr 2 teach lstk update to handle the bundled extensions by carillan81 · Pull Request #482 · localstack/lstk

carillan81 · 2026-09-03T15:19:14Z

Motivation

Once releases ship the bundled extensions (bundled-extensions and lstk-extensions.toml next to lstk), lstk update on the binary channel has to replace all three files together, not just the binary. Today it replaces one file, lstk, and ignores everything else in the archive.

That has a second-order problem. The first bundling release is installed by the updater users already have, which only replaces lstk. They land on a current binary with no extensions, and because they are already on the newest version, lstk update says "already up to date" until the next release ships. Nobody can fix that retroactively, so the new updater has to detect and repair it.

Solution

Set-wise replacement (stage-then-commit). Every member of the archive is copied into the install directory under a .lstk-new name; only when all copies succeed is each renamed over its final name, lstk last. A file visible under its real name is never half-written, a failure before the last rename leaves a working lstk to re-run with, and the updater never deletes an lstk-* file it cannot prove it owns (additive-only). An archive containing only lstk behaves exactly as before, so pre-bundling releases and rollbacks are unaffected.

Repair when current but incomplete. The release stamps the expected set into the binary (version.bundledSet, via ldflags). When versions match but a stamped member is missing or unusable, lstk update reinstalls the same release, then re-checks; if the archive still did not deliver the members it fails loudly ("did not restore the bundled extensions") instead of looping. Empty stamp keeps today's pure version comparison. Homebrew and npm never repair; the package manager owns the whole set there. The passive update notice also nudges an incomplete install toward lstk update.

The same-version repair from the first revision was dropped in review. The transition release (old updater, new archive) leaves the bundle uninstalled until the next lstk update. Instead of repairing, lstk tells the user: a release build that finds no bundle beside itself adds a reinstall hint to the unknown-command error and warns on an up-to-date lstk update (also in the --json warnings array). One goreleaser ldflag, one detector, two call sites.

Hardening. Windows moves every existing member aside before renaming (a running extension no longer breaks the commit); a squatting symlink or directory at a staging path is refused rather than written through; the install directory is listed literally instead of globbed (a [ in the path used to fail every update); setuid/setgid bits survive the update; a concurrent update cannot truncate another's staging file.

Structured output. UpdateCheckedEvent gains RepairBundled; the --json check shape gains "repairBundled": true for a same-version repair (absent otherwise, and scrubbed from the applied shape).

Dependencies and follow-ups

dpx-692 (PR 1) must merge first or together: it carries the resolver that dispatches bundled commands from the toml. Without it a repaired install has the files but nothing resolves them.
The bundledSet ldflags stamp (tasks.md 5.2) is not yet in .goreleaser.yaml; until it lands on the packaging side the repair path stays dormant and this PR changes nothing user-visible.
Once both branches are in, alias bundledBinaryBaseName from extension.BundledBinaryName (TODO left on the constant).

Testing

Unit: internal/update covers every behaviour above, including the Windows path from any host via a goos parameter. All new behaviour was written test-first.
Integration: transition repair, complete-set no-op, pre-bundling release unaffected, broken-archive fail-loud, leftover cleanup, plus the existing mock-GitHub happy/failure paths.
Manual testbed against packages built by the dpx-692 pipeline (goreleaser, npm publisher): the full transition (published 0.23.0 to bundling release, repair, no-op, bundled-to-bundled upgrade, rollback, forward) passes on the binary channel and npm. Caveat: the currently published bundle predates the bundled-extensions list contract, so the release gate was run as a warning for that exercise; Homebrew was not executed.

Docs

No documentation work needed outside this PR. docs/structured-output.md is updated for the new repairBundled key, and the update guarantees are documented in the internal/update package comment. User-facing bundling docs (docs/extensions-bundling.md) live on dpx-692.

The reinstall hint is new user-facing output; the spec has a scenario for it. Release notes for the first bundling release should carry one line: if a bundled command reports "unknown command" after updating, reinstall lstk once.

Review

Human review advisable. This is the update path, the one thing a broken release cannot ship a fix for, and it adds new user-visible behaviour (the repair). Not a self-merge candidate.

Closes DPX-693

Co-Authored-By: Claude <noreply@anthropic.com>

…-bundle

peter-smith-phd

Thanks for implementing this, but I'm not sure we need a 2000-line solution to handle the "binary" case where we're moving from just lstk to lstk/bundled-extensions/lstk-extensions.toml. I appreciate the need to carefully replace the three files, rather than just one, but this solution feels over complicated. I was expecting closer to 100 lines of code.

In particular, do we really need the "repair" functionality? If it breaks for some reason, we already have error messages that inform the user of the problem. They should instead just uninstall/reinstall lstk and the problem will go away. My concern is that the code has become so complicated that we'll have trouble maintaining it in future.

What can we do to make this significantly less complicated, and focus on just replacing three files instead of one?

…ser. Co-Authored-By: Claude <noreply@anthropic.com>

carillan81 · 2026-09-04T11:43:41Z

Thanks for implementing this, but I'm not sure we need a 2000-line solution to handle the "binary" case where we're moving from just lstk to lstk/bundled-extensions/lstk-extensions.toml. I appreciate the need to carefully replace the three files, rather than just one, but this solution feels over complicated. I was expecting closer to 100 lines of code.

In particular, do we really need the "repair" functionality? If it breaks for some reason, we already have error messages that inform the user of the problem. They should instead just uninstall/reinstall lstk and the problem will go away. My concern is that the code has become so complicated that we'll have trouble maintaining it in future.

What can we do to make this significantly less complicated, and focus on just replacing three files instead of one?

After some discussion with humans and AI I agree that the repair functionality is an overkill. We are facing a temporal issue while the users update and the repair adds some code that will be stale in a month.

I think we must address user UX so I just implemented a better error message that gives a hint for reinstall (and command) in case the command is not found. It adds some code but barely adds a few tenths of lines.

Bucket	Code	Comments	Blank	Docs / config	Added	Removed
Go production	247	82	24		353	29
Go tests	679	46	62		787	51
Docs (openspec spec + tasks, CLAUDE.md)				16	16	16
Build config (`.goreleaser.yaml`)				1	1	1
Total	926	128	86	17	1157	97

This looks a much better solution to me.

$ lstk deploy
Error: unknown command "deploy" for lstk
  This lstk release ships bundled extensions, but none are installed in <install dir>.
  ==> See help: lstk -h
  ==> Reinstall lstk: download the latest release from https://github.com/localstack/lstk/releases/latest
(exit 1)

$ lstk update
Checking for updates...
> Note: Already up to date (9.9.9-SNAPSHOT-dfd62a7)
> Warning: This lstk release ships bundled extensions, but none are installed in <install dir>. Reinstall lstk: download ...

Does this works for you?

peter-smith-phd · 2026-09-07T00:44:16Z

Does this works for you?

Sorry, I ran out of time to review this today, but will look at it in more detail tomorrow. On the surface, it looks like a good improvement 😄

peter-smith-phd · 2026-09-07T21:43:23Z

 - **THEN** the update succeeds using the in-the-field updater (which replaces only the lstk binary and ignores the archive's extra members)
 - **AND** the install is left with an incomplete set, since that updater predates bundling and cannot be made to fail
- **AND** the incomplete set is repaired by the next `lstk update`, which SHALL NOT wait for a newer release to become available
+- **AND** until the next release's `lstk update` (now the set-wise updater) installs the complete set, lstk points the user at a reinstall: an unknown command and an up-to-date `lstk update` both say the release ships bundled extensions that are not installed, and name the reinstall command for the install method (accepted one-release gap; a same-version repair was considered and dropped as too much machinery in PR #482)


This doesn't feel correct. Will the following behaviour happen?

I update next week (using the binary install method) which doesn't (yet) download the bundled-extensions binary too.

I type lstk strt (note the typo)

It suggests "you should reinstall lstk"?

Am I understanding that correctly? I would have thought that lstk would just say Error: unknown command "strt" for lstk as it does now, since reinstalling seems pretty drastic to recover from a typo. The issue of the missing extensions will automatically fix itself in the next update.

In a month or so we can change this behaviour, but it doesn't feel correct in the migration period.

peter-smith-phd · 2026-09-07T21:47:12Z


+// bundlesExtensions is "true" on release builds, which ship the
+// bundled-extensions binary and lstk-extensions.toml beside lstk.
+var bundlesExtensions = "false"


Won't this always be true from now on? Is there a reason that the BundlesExtensions function must be called? Why not just assume that all future releases have bundles installed?

this is false until we merge the other PR, just in case we merge this one first (this one could already be merged and released with lstk usual cycle)

peter-smith-phd · 2026-09-07T21:49:40Z

+func ReinstallCommand(m InstallMethod) string {
+	switch m {
+	case InstallHomebrew:
+		return "brew reinstall --cask localstack/tap/lstk"


I had the impression that brew and npm would handle the "set" correctly, and we wouldn't need to detect missing files.

peter-smith-phd · 2026-09-07T21:52:28Z

      - -trimpath
    ldflags:
-      - -s -w -X github.com/localstack/lstk/internal/version.version={{ .Version }}
+      - -s -w -X github.com/localstack/lstk/internal/version.version={{ .Version }} -X github.com/localstack/lstk/internal/version.bundlesExtensions=true


As mentioned earlier, I'm unsure why this is necessary. I don't think we plan to stop having bundles, so couldn't the code simply hard-code the fact that all lstk releases use bundled extensions.

peter-smith-phd · 2026-09-07T21:56:56Z

+//     lstk commits last, so a working lstk always remains. Windows caveat: a
+//     crash between renaming lstk.exe aside and renaming the new one in leaves
+//     no lstk.exe; rename lstk.exe.old back by hand.
+//  3. Nothing is deleted: an lstk-* file absent from the archive is left alone,


This third bullet point doesn't seem relevant anymore, given that we don't use lstk-* files anymore.

peter-smith-phd · 2026-09-07T21:59:22Z

+//
+// An archive carrying only lstk installs exactly as before bundling existed.
+//
+// The pre-bundling updater installs a bundling release with only its lstk


Now that I think about it - this "update" process is what should really be failing if the "bundle" isn't downloaded correctly. The whole idea of checking (DetectMissingBundle) when the user later runs a command seems a bit late. It's very unlikely that the bundle files will suddenly go missing after they're successfully installed.

peter-smith-phd · 2026-09-07T22:00:19Z

+}
+
+// extractAndReplace installs every member the archive carries as one unit:
+// lstk, the bundled-extensions binary, any lstk-* binaries, and the


Comment is out of date. We no longer have lstk-* binaries.

peter-smith-phd · 2026-09-07T22:23:28Z

+
+// replaceSet takes the platform as a parameter so the Windows naming and
+// move-aside rules are testable on any host (unit tests run on Linux in CI).
+func replaceSet(archivePath, exePath, format, goos string) error {


While I was reviewing this, I started to wonder why we need to do this file-by-file. Since the binary archives are literally tar.gz or .zip files, why couldn't we just extract the full archive and replace the entire directory in a single mv command. Unfortunately, I think the answer is that we don't have a well-defined way for binary-downloads to be installed, and we can't possibly know where the user decided to install lstk in the first place. This is messy, but that's what we have, so I guess the file-by-file replacement makes sense and partly explains why this PR is longer than I thought it should be 🫨

But, it did uncover the fact that completions and manpages directories are never updated. They'll always contain the original content they were downloaded with (at least, that's what Claude told me).

Carlos Arilla and others added 2 commits September 2, 2026 12:06

Replace the whole bundled set on update, not just the lstk binary

277943f

Co-Authored-By: Claude <noreply@anthropic.com>

Repair incomplete bundled sets on update and harden the staging path

7038154

Co-Authored-By: Claude <noreply@anthropic.com>

carillan81 added semver: minor docs: skip Pull request does not require documentation changes labels Sep 3, 2026

Merge branch 'main' into dpx-693-pr-2-teach-lstk-update-to-handle-the…

6dca483

…-bundle

carillan81 marked this pull request as ready for review September 3, 2026 18:08

carillan81 requested review from a team and peter-smith-phd as code owners September 3, 2026 18:08

peter-smith-phd requested changes Sep 3, 2026

View reviewed changes

Remove reinstall/repair process, give better error and hints to the u…

7fcf2d4

…ser. Co-Authored-By: Claude <noreply@anthropic.com>

carillan81 requested a review from peter-smith-phd September 4, 2026 12:41

peter-smith-phd reviewed Sep 7, 2026

View reviewed changes

Navigation Menu

Uh oh!

Dpx 693 pr 2 teach lstk update to handle the bundled extensions - #482

Dpx 693 pr 2 teach lstk update to handle the bundled extensions#482
carillan81 wants to merge 4 commits into
mainfrom
dpx-693-pr-2-teach-lstk-update-to-handle-the-bundle

carillan81 commented Sep 3, 2026 •

edited

Loading

Uh oh!

peter-smith-phd left a comment

Uh oh!

carillan81 commented Sep 4, 2026 •

edited

Loading

Uh oh!

peter-smith-phd commented Sep 7, 2026

Uh oh!

peter-smith-phd Sep 7, 2026

Uh oh!

peter-smith-phd Sep 7, 2026

Uh oh!

carillan81 Sep 8, 2026

Uh oh!

peter-smith-phd Sep 7, 2026

Uh oh!

peter-smith-phd Sep 7, 2026

Uh oh!

peter-smith-phd Sep 7, 2026

Uh oh!

peter-smith-phd Sep 7, 2026

Uh oh!

peter-smith-phd Sep 7, 2026

Uh oh!

peter-smith-phd Sep 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

carillan81 commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Solution

Review

Uh oh!

peter-smith-phd left a comment

Choose a reason for hiding this comment

Uh oh!

carillan81 commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peter-smith-phd commented Sep 7, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

carillan81 commented Sep 3, 2026 •

edited

Loading

carillan81 commented Sep 4, 2026 •

edited

Loading